hisilicon-osdrv-hi3516cv200: add GC2023 MIPI sensor variant by phedoreanu · Pull Request #2248 · OpenIPC/firmware

phedoreanu · 2026-08-09T16:30:20Z

Some GC2023 boards wire the sensor to MIPI CSI-2, not DVP — e.g. VStarcam Hi3518EV200 pan/tilt cameras (2-lane RAW10). For those boards the existing gc2023 entry in load_hisilicon is fatal: it muxes the parallel VI pads (0x200f007c..0x200f0094), which disables the MIPI PHY. The tell is /proc/umap/vi showing IntCnt=0 with TmgErr=0 and ccErrN=0 — VI receives no VSYNC at all, while bad ini timings would at least raise the error counters.

Following the OV2710 precedent (#2035/#2038), this adds:

a gc2023_mipi sensor identity that only muxes I2C0 and sets the 24 MHz sensor clock — the plain gc2023 DVP entry is untouched;
gc2023_mipi_1080p.ini, cloned from ov2735_mipi_1080p.ini (same SoC, same 2-lane RAW10 MIPI geometry) with the sensor identity swapped and Isp_Bayer corrected to 0 (RGGB — GC2023 is SRGGB10; the symptom of getting this wrong is red and blue swapping).

Verified on a VStarcam Hi3518EV200: with this identity and ini, VI IntCnt climbs and majestic delivers 1920×1080 at the sensor's native timing.

One honest caveat, detailed in #2243: the shipped closed-source libsns_gc2023.so did not produce frames on the tested unit (VENC timeout; it appears to program the sensor for DVP output) — the working setup used the vendor's MIPI libsns plus an init-table replay. The platform side fixed here is required either way, and #2243 carries the full recovered init table (123 writes, including the digital-gain 0xb1 trap) toward an open-source driver.

qodo-free-for-open-source-projects · 2026-08-09T16:31:37Z

PR Summary by Qodo

Add GC2023 MIPI (CSI-2) sensor variant for Hi3516CV200

✨ Enhancement ⚙️ Configuration changes 🕐 10-20 Minutes

AI Description

• Add new gc2023_mipi sensor identity to preserve MIPI pinmux/PHY operation.
• Introduce a 1080p MIPI RAW10 INI profile with correct GC2023 Bayer order.
• Keep existing gc2023 (DVP) behavior unchanged for parallel-wired boards.

Diagram

graph TD
  A[/"User selects sensor: gc2023_mipi"/] --> B["load_hisilicon (insert_sns)" ] --> C[("SoC pinmux/clock regs")]
  B --> D[/"gc2023_mipi_1080p.ini"/]
  D --> E["libsns_gc2023.so" ] --> F(("GC2023 sensor"))
  C --> G["VI + MIPI CSI-2" ]
  F --> G

  subgraph Legend
    direction LR
    _cfg[/"Config (.ini/env)"/] ~~~ _svc["Script/driver glue" ] ~~~ _hw[("HW regs") ] ~~~ _sns(("Sensor") )
  end

High-Level Assessment

The following are alternative approaches to this PR:

1. Auto-detect MIPI vs DVP at boot

➕ No need for separate gc2023_mipi identity; fewer user-facing sensor names.
➕ Could reduce misconfiguration risk on unknown boards.
➖ Detection is platform- and driver-dependent (PHY status, /proc counters), potentially flaky early-boot.
➖ Adds complexity to a critical init script path and raises risk of false positives.

2. Make `gc2023` conditional on INI input_mode

➕ Single sensor name while allowing both MIPI and DVP wiring.
➕ Keeps configuration centralized in INI.
➖ load_hisilicon currently keys off sensor identity and does not parse INI; would require new parsing logic or refactor.
➖ More invasive change with higher regression potential for existing boards.

Recommendation: Keep the PR’s approach (add a distinct gc2023_mipi identity plus a matching INI). It’s the lowest-risk fix because it avoids changing the existing gc2023 (DVP) behavior while preventing the fatal DVP pad muxing on MIPI-wired boards. Alternatives (auto-detection or INI-driven conditional pinmux) are feasible but would be more complex and riskier in early-boot hardware init.

Files changed (2) +290 / -2

Enhancement (1) +6 / -2

load_hisiliconAdd 'gc2023_mipi' to MIPI-safe pinmux/clock init path +6/-2

Add 'gc2023_mipi' to MIPI-safe pinmux/clock init path
• Extends the MIPI-variant sensor case list to include 'gc2023_mipi', ensuring only I2C0 muxing and sensor clock setup are applied. Adds inline documentation explaining that the existing 'gc2023' entry muxes DVP pads and can disable the MIPI PHY on CSI-2-wired boards.
general/package/hisilicon-osdrv-hi3516cv200/files/script/load_hisilicon

Other (1) +284 / -0

gc2023_mipi_1080p.iniIntroduce GC2023 MIPI 1080p RAW10 sensor profile +284/-0

Introduce GC2023 MIPI 1080p RAW10 sensor profile
• Adds a new INI profile for GC2023 over 2-lane MIPI CSI-2 at 1920x1080, including lane configuration and VI/VPSS/VENC defaults. Sets 'Isp_Bayer=0' (RGGB) to match GC2023 (SRGGB10) and avoid red/blue channel swapping.
general/package/hisilicon-osdrv-hi3516cv200/files/sensor/config/gc2023_mipi_1080p.ini

qodo-free-for-open-source-projects · 2026-08-09T16:33:30Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can ask Qodo to dismiss a finding you disagree with, with your reason on record

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

flyrouter

Thanks.
It looks like an interesting proposal for improvement.

widgetii

Thanks — this is unusually well-evidenced work, and the writeup in #2243 is genuinely useful material. I reviewed both halves separately, because they have very different risk and value profiles.

The `load_hisilicon` half: correct, and I want it

No regression risk, verified rather than assumed:

case matching is exact-glob and the DVP gc2023 arm comes first, so no existing sensor value changes behaviour.
Kernel side: open_sys_config for cv200 carries its own duplicate pinmux table (kernel/sys_config/hi3516cv200/sys_config.c) and falls through to printk("sensor_type '%s' is error!!!") for names it does not know — non-fatal, and the script's devmem writes run afterwards and win. gc2023, ov2735 and several others already hit that same branch today.
The vendor hi3518e_sys.ko takes sensor as a bare charp with no name table, so an unknown value is inert there too.
Affected defconfigs are hi3516cv200_lite, hi3518ev200_lite, hi3518ev200_ultimate, hi3518ev201_lite. The new .ini is installed unconditionally by the *.ini glob in the .mk: ~10 KB raw, ~3 KB compressed. Irrelevant even on the tight lite boards.

One asymmetry worth flagging: the OV2710 precedent you follow (#2038) updated both sys_config.c in openhisilicon and load_hisilicon. This PR only does the latter. That is harmless, but it means dmesg now gains a sensor_type 'gc2023_mipi' is error!!! line, which looks alarming to anyone reading a boot log. Adding the arm to sys_config.c as well would keep the two tables in sync and keep the log clean.

The `.ini` half: one real objection, and it is the header

From your own table in #2243, on the same unit with the same MIPI ini and the same fixed pinmux:

library	frames
vendor MIPI `libsns_gc2023.so`	yes
vendor DVP `libsns_gc2023_dvp.so`	none (VENC timeout)
OpenIPC `libsns_gc2023.so`	none (VENC timeout)

DllFile in this file points at row three. So anyone who follows this file's own instructions gets a VENC timeout. That is fine — it is a platform-side template, and it is useful to someone who supplies their own MIPI libsns and edits one line — but the header currently says "Verified on a VStarcam Hi3518EV200 board with the GC2023 wired to MIPI", which is not true of this file with this DllFile. Verification was done with a library that is not the one named here. That comment will cost somebody a day.

Before merge I would like just the header corrected (suggestion inline). The frame-rate and Bayer notes are questions, not blockers.

On "what is the point if the camera still does not work"

Answering it here since it will come up. The platform half is a prerequisite that no library can work without — without it the MIPI PHY is dead and the sensor is unreachable on these boards regardless of what userspace you throw at it — and it costs nothing. The .ini is scaffolding that the open driver your init table in #2243 makes possible will plug straight into. Both are worth having, as long as the file is honest about what does and does not work today. That is the whole of my ask.

phedoreanu · 2026-08-12T08:31:25Z

All asks addressed, and the naming experiment produced a more interesting answer than either of us guessed.

Header (b168796): your text taken essentially verbatim. The file now leads with the NOTE that the OpenIPC-shipped libsns_gc2023.so does not stream on a MIPI-wired board and that DllFile must point at a GC2023 MIPI library OpenIPC does not currently ship; verification credit goes to the vendor MIPI libsns, with a pointer to #2243. The same commit adds the Isp_FrameRate note and fixes the "below"/"above" slip in the load_hisilicon comment.

The naming experiment, run on the unit:

# fw_setenv sensor gc2023_mipi
# ipcinfo -s
gc2023
# majestic -s   (foreground)
DEBUG [sdk] try_sensor_config@5428: Autodetected sensor as 'gc2023_i2c'
DEBUG [sdk] find_sensor_config@5388: matched sensor config: gc2023_i2c_1080p.ini

The search key does not come from the sensor env at all: majestic autodetects by I2C probe and derives gc2023_i2c even with sensor=gc2023_mipi set. So gc2023_mipi_i2c_1080p.ini would not auto-resolve either — the only auto-matching name is a gc2023_i2c* prefix, and that key cannot distinguish a MIPI-wired board from a DVP-wired one (the suffix records how majestic talks to the sensor, not how video leaves it). Since this file needs a manually supplied MIPI libsns anyway, staying off the auto-match path and requiring the explicit cli -s seems right — the same situation ov2710_mipi_1080p.ini / ov2710_dc_1080p.ini are in against their probed ov2710_i2c key. Name kept.

sys_config.c asymmetry: fixed for real rather than flagged — OpenIPC/openhisilicon#207 adds the gc2023_mipi arm as a mirror of the ov2710_mipi one, so a gc2023_mipi boot loses the sensor_type 'gc2023_mipi' is error!!! line and the two pinmux tables stay in sync.

Isp_FrameRate 25 vs 30: a leftover of the clone — but a leftover the verification ran with. The working vendor-libsns session used this file with 25 in place, while the recovered init table is sensor_linear_1080p30_init. Your theory fits the AE oddities in #2243 (MaxLine=2, Error=-196), so I noted the tension next to the value rather than silently flipping to an untested 30; it should be revisited together with the AE limits once an open MIPI driver exists (#2243 carries the full init table for that).

And thank you for the closing paragraph — "prerequisite plus scaffolding, as long as the file is honest" is exactly the framing I will steal for the next one of these.

Mirror of the ov2710_mipi arm: I2C0 pinmux plus 24MHz sensor clock, with the parallel VI pads left alone so the MIPI PHY keeps the pins. Companion to OpenIPC/firmware#2248, which adds the gc2023_mipi identity to load_hisilicon and a gc2023_mipi_1080p.ini; without this arm a gc2023_mipi boot logs "sensor_type 'gc2023_mipi' is error!!!" before the script's devmem writes apply the same values. The register writes themselves are hardware-verified: they are the ones the load_hisilicon arm issues via devmem on a VStarcam Hi3518EV200 (see OpenIPC/firmware#2243 for the bring-up record).

Some GC2023 boards wire the sensor to the MIPI CSI-2 interface, not DVP - e.g. VStarcam Hi3518EV200 cameras (2-lane RAW10). For those boards the existing gc2023 entry in load_hisilicon is fatal: it muxes the parallel VI pads (0x200f007c..0x200f0094) to VI_DATA/VI_VS/VI_HS, which disables the MIPI PHY. The tell is /proc/umap/vi showing IntCnt=0 with TmgErr=0 and ccErrN=0 - VI receives no VSYNC at all, while bad ini timings would at least raise the error counters. Following the OV2710 precedent (OpenIPC#2035/OpenIPC#2038), add a gc2023_mipi sensor identity that only muxes I2C0 and sets the 24MHz sensor clock, plus a gc2023_mipi_1080p.ini cloned from ov2735_mipi_1080p.ini (same SoC, same 2-lane RAW10 MIPI geometry) with the sensor identity swapped and Isp_Bayer corrected to 0 (RGGB - GC2023 is SRGGB10; the tell for getting this wrong is red and blue swapping). The plain gc2023 DVP entry is left untouched. Verified on a VStarcam Hi3518EV200: with this identity and ini, VI IntCnt climbs and majestic delivers 1920x1080 at the sensor's native timing. Note the shipped closed-source libsns_gc2023.so did not produce frames on that unit (VENC timeout; it appears to program the sensor for DVP output) - the working setup used the vendor's MIPI libsns. The platform side this commit fixes is required either way, and the sensor init details recovered from the vendor lib are documented in the matching issue for a future open-source driver.

Review follow-up. The ini header claimed 'verified on a VStarcam Hi3518EV200' while its own DllFile points at the OpenIPC-shipped libsns_gc2023.so, which produces no frames on a MIPI-wired board (VENC timeout - it appears to program the sensor for DVP output). The MIPI geometry was in fact validated with the vendor's MIPI library. Say so in the header, so nobody selects this preset and burns a day chasing optics; point at issue OpenIPC#2243 for the full bring-up record. Also note next to Isp_FrameRate that 25 is the value the verification ran with while the vendor init table is a 1080p30 one, and fix the load_hisilicon comment that pointed at the plain gc2023 arm as 'below' when it sits in the first case arm above.

phedoreanu requested review from cronyx, flyrouter and viktorxda as code owners August 9, 2026 16:30

flyrouter reviewed Aug 11, 2026

View reviewed changes

widgetii reviewed Aug 12, 2026

View reviewed changes

phedoreanu mentioned this pull request Aug 12, 2026

kernel/hi3516cv200: recognize the gc2023_mipi sensor type OpenIPC/openhisilicon#207

Merged

phedoreanu force-pushed the gc2023-mipi branch from b168796 to 7b88d55 Compare August 12, 2026 08:39

phedoreanu added 2 commits August 15, 2026 16:23

widgetii force-pushed the gc2023-mipi branch from 7b88d55 to 5c4fbf9 Compare August 15, 2026 13:23

widgetii merged commit 991420e into OpenIPC:master Aug 15, 2026
102 checks passed

Navigation Menu

Uh oh!

Uh oh!

hisilicon-osdrv-hi3516cv200: add GC2023 MIPI sensor variant - #2248

hisilicon-osdrv-hi3516cv200: add GC2023 MIPI sensor variant#2248
widgetii merged 2 commits into
OpenIPC:masterfrom
phedoreanu:gc2023-mipi

phedoreanu commented Aug 9, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 9, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 9, 2026

Uh oh!

flyrouter left a comment

Uh oh!

widgetii left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

phedoreanu commented Aug 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

phedoreanu commented Aug 9, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 9, 2026

PR Summary by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 9, 2026

Code Review by Qodo

Great, no issues found!

Uh oh!

flyrouter left a comment

Choose a reason for hiding this comment

Uh oh!

widgetii left a comment

Choose a reason for hiding this comment

The load_hisilicon half: correct, and I want it

The .ini half: one real objection, and it is the header

On "what is the point if the camera still does not work"

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

phedoreanu commented Aug 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

The `load_hisilicon` half: correct, and I want it

The `.ini` half: one real objection, and it is the header